WIP: Add --unified to use our container-storage for host images #1601
Conversation
| /// Use podman/skopeo to pull image to additionalimagestore, then read from container storage. | ||
| /// This provides a unified approach that leverages existing container tooling. | ||
| #[clap(long)] | ||
| pub(crate) unified: bool, |
There was a problem hiding this comment.
I don't think this should be a CLI flag that operates just once; it should be something like bootc image --set-unified or so and act persistently.
Also we should support setting this at install time so that it happens from the very start.
There was a problem hiding this comment.
(hmm my bad I thought we discussed this but I failed to update the issue #20 or something maybe?)
There was a problem hiding this comment.
Ahh man yeah, we talked about saving the config on the origin file IIRC, I forgot.
| use bootc_utils::CommandRunExt; | ||
|
|
||
| // Use podman pull with additionalimagestore pointing to bootc storage | ||
| let bootc_storage_path = "/usr/lib/bootc/storage"; |
There was a problem hiding this comment.
Use podstorage.rs instead please
There was a problem hiding this comment.
To elaborate on this, an important aspect here is currently the GC of the bootc/storage instance is rooted in the set of LBIs. That will need to be extended to include the host image.
There was a problem hiding this comment.
Another big task related to this - we'll eventually really want to use the podman HTTP APIs so we can properly monitor progress. Right now we are very crudely relying on doing this synchronously to the existing tty and reusing the podman CLI tty output. But that can't really work if we ever need to render our own progress APIs.
https://lib.rs/crates/bollard might be a good choice for this?
| match prepare_for_pull_unified(repo, imgref, target_imgref, store).await? { | ||
| PreparedPullResult::AlreadyPresent(existing) => { | ||
| // Log that the image was already present (Debug level since it's not actionable) | ||
| const IMAGE_ALREADY_PRESENT_ID: &str = "5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9"; |
There was a problem hiding this comment.
Note that if we go this route, we should also not log to the journal in the ostree pull path because otherwise we're double logging.
| let fetched = if opts.unified { | ||
| crate::deploy::pull_unified(repo, imgref, None, opts.quiet, prog.clone(), sysroot).await? | ||
| } else { | ||
| crate::deploy::pull(repo, imgref, None, opts.quiet, prog.clone()).await? |
There was a problem hiding this comment.
This relates to #1599 (comment)
Basically how about changing this code to take a Storage which would hold the persistent flag, and then crate::deploy::pull would itself query that flag and change its behavior.
That would also implicitly then fix the install path to behave the same.
|
Do we get reflinks when importing from the alternate storage into the ostree repo? |
#20 links to containers/container-libs#144 |
Hmm, that's about copying between two container storages, but I was asking about the ostree import. But #20 does say:
Which kinda answers the question. Basically without reflinks into the ostree storage, IMO this makes it a lot less obvious that this new option is objectively better and it instead becomes a discussion of trade-offs. |
Compiles and at least does not fail a bootc switch... and can see the image in the container storage we own.
This was drafted with the help of Claude Code.